Bug t radiative #2560

Sumit112192 · 2024-03-23T09:16:51Z

📝 Description

Type: 🪲 bugfix| memo: documentation

ref. https://tardis-sn.github.io/tardis/physics/setup/model.html#Temperatures

tardis/tardis/model/parse_input.py

Lines 726 to 731 in 1718002

    
           lambda_wien_inner = const.b_wien / packet_source.temperature 
        
           t_radiative = const.b_wien / ( 
        
               lambda_wien_inner 
        
               * (1 + (geometry.v_middle - geometry.v_inner_boundary) / const.c) 
        
           ) 
        
           return t_radiative

The code doesn't match with the documentation. So, I have changed it accordingly.
I have directly used packet_source.temperature rather than first computing lambda_wien_inner

Slight documentation typos:
The documentation string of parse_structure_config was repeated twice, so I removed the extra one.
Also, there were some instances where v_inner, v_outer were labeled as r_inner, r_outer. So, I have changed them as well.

Also, If we pass packet_source as one of the function arguments, why define it again in the body?

tardis/tardis/model/parse_input.py

Lines 589 to 618 in 1718002

    
           def initialize_packet_source(config, geometry, packet_source): 
        
               """ 
        
               Initialize the packet source based on config and geometry 
        
               Parameters 
        
               ---------- 
        
               config : Config 
        
                   The configuration object containing the supernova and plasma settings. 
        
               geometry : Geometry 
        
                   The geometry object containing the inner radius information. 
        
               packet_source : BasePacketSource 
        
                   The packet source object based on the configuration and geometry. 
        
               Returns 
        
               ------- 
        
               packet_source : BasePacketSource 
        
                   The packet source object based on the configuration and geometry. 
        
               Raises 
        
               ------ 
        
               ValueError 
        
                   If both t_inner and luminosity_requested are None. 
        
               """ 
        
               if config.montecarlo.enable_full_relativity: 
        
                   packet_source = BlackBodySimpleSourceRelativistic( 
        
                       base_seed=config.montecarlo.seed, 
        
                       time_explosion=config.supernova.time_explosion, 
        
                   ) 
        
               else: 
        
                   packet_source = BlackBodySimpleSource(base_seed=config.montecarlo.seed)

📌 Resources

Examples, notebooks, and links to useful references.

🚦 Testing

How did you test these changes?

Testing pipeline
Other method (describe)
My changes can't be tested (explain why)

☑️ Checklist

I requested two reviewers for this pull request
I updated the documentation according to my changes
I built the documentation by applying the build_docs label

Note: If you are not allowed to perform any of these actions, ping (@) a contributor.
@andrewfullard @jamesgillanders Please review this pull request.

andrewfullard · 2024-04-15T14:19:02Z

tardis/model/parse_input.py

-        lambda_wien_inner
-        * (1 + (geometry.v_middle - geometry.v_inner_boundary) / const.c)
+    t_radiative = packet_source.temperature / (
+        (1 - (geometry.v_middle - geometry.v_inner_boundary) / const.c)


The documentation is incorrect here: the 1 - should be 1 + because temperature should decrease further out. The other change (packet_source.temperature) is fine.

You can modify the documentation to fix the sign of the function

Yes !!! That makes sense. I will change the documentation and will redo this change to the original.

DeerWhale · 2024-04-15T15:11:25Z

Thanks for spotting the duplication of the packet initialization, I have opened an issue #2585 and our team members will look into this further. Feel free to dive deeper if you are interested as well!

…Bug_T_radiative

review-notebook-app · 2024-05-02T21:25:14Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

Sumit112192 · 2024-05-02T21:34:09Z

@andrewfullard I have made the necessary changes.

The changes that I have made till now.

Updated the documentation so that doppler factor is $1+\frac{\Delta v}{v}$ instead of $1-\frac{\Delta v}{v}$
Fixed some common typos in the documentation strings of the functions
Change suggested by Black in the file tardis/model/parse_input.py

Sumit112192 · 2024-05-02T21:37:13Z

I don't know why Black's test is failing on tardis/model/parse_input.py, but I have made the change that Black suggested.

Sumit112192 · 2024-05-02T21:42:25Z

It seems that the suggestions by black installed through sudo apt install black on Ubuntu are different from those of black installed through pip install black.

Sumit112192 · 2024-05-02T21:55:38Z

Sorry for all those commits. I am still trying to figure out how branching works in Git, and hopefully, I will not repeat the same mistake.

codecov · 2024-05-03T00:22:04Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.18%. Comparing base (ea5a1bf) to head (a0f2dad).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2560      +/-   ##
==========================================
- Coverage   67.18%   67.18%   -0.01%     
==========================================
  Files         173      173              
  Lines       14570    14569       -1     
==========================================
- Hits         9789     9788       -1     
  Misses       4781     4781

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

andrewfullard · 2024-05-03T02:01:25Z

It seems that the suggestions by black installed through sudo apt install black on Ubuntu are different from those of black installed through pip install black.

Make sure you use the version of black that is installed with the TARDIS environment

tardis-bot · 2024-05-03T12:46:19Z

*beep* *bop*

Hi, human.

The docs workflow has succeeded ✔️

Click here to see your results.

tardis/model/parse_input.py

Sumit112192 added 4 commits March 23, 2024 11:02

Fix in parse_structure_config documentation string

109df81

Documentation Typo

6ea9fba

Another type

f9fa649

Fix

66c4283

andrewfullard requested a review from wkerzendorf March 25, 2024 13:23

andrewfullard added the documentation label Apr 9, 2024

andrewfullard reviewed Apr 15, 2024

View reviewed changes

Sumit112192 and others added 9 commits May 3, 2024 02:27

Fix in parse_structure_config documentation string

e42b6a0

Documentation Typo

8efec58

Another type

b4ceb53

Fix

035c789

Redo the change to original and updated the documentation

b516b6a

Merge branch 'tardis-sn:master' into Bug_T_radiative

288a473

Redo the change to original and updated the documentation

4f308a2

Merge branch 'Bug_T_radiative' of github.com:Sumit112192/tardis into …

e1b3ef1

…Bug_T_radiative

Undo the original

62a4346

Spelling Typo

ff52279

Undo the change by black

a0f2dad

andrewfullard self-requested a review May 3, 2024 12:28

andrewfullard added the build-docs label May 3, 2024

andrewfullard approved these changes May 3, 2024

View reviewed changes

DeerWhale reviewed May 6, 2024

View reviewed changes

tardis/model/parse_input.py Show resolved Hide resolved

andrewfullard requested a review from DeerWhale May 8, 2024 17:47

andrewfullard enabled auto-merge (squash) May 10, 2024 14:12

Knights-Templars approved these changes May 10, 2024

View reviewed changes

andrewfullard merged commit 8d70aaa into tardis-sn:master May 10, 2024
13 checks passed

Sumit112192 deleted the Bug_T_radiative branch May 10, 2024 14:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug t radiative #2560

Bug t radiative #2560

Sumit112192 commented Mar 23, 2024 •

edited

Loading

andrewfullard Apr 15, 2024

andrewfullard Apr 15, 2024

Sumit112192 Apr 15, 2024

DeerWhale commented Apr 15, 2024

review-notebook-app bot commented May 2, 2024

Sumit112192 commented May 2, 2024

Sumit112192 commented May 2, 2024

Sumit112192 commented May 2, 2024

Sumit112192 commented May 2, 2024

codecov bot commented May 3, 2024

andrewfullard commented May 3, 2024

tardis-bot commented May 3, 2024

	lambda_wien_inner = const.b_wien / packet_source.temperature
	t_radiative = const.b_wien / (
	lambda_wien_inner
	* (1 + (geometry.v_middle - geometry.v_inner_boundary) / const.c)
	)
	return t_radiative

	def initialize_packet_source(config, geometry, packet_source):
	"""
	Initialize the packet source based on config and geometry

	Parameters
	----------
	config : Config
	The configuration object containing the supernova and plasma settings.
	geometry : Geometry
	The geometry object containing the inner radius information.
	packet_source : BasePacketSource
	The packet source object based on the configuration and geometry.

	Returns
	-------
	packet_source : BasePacketSource
	The packet source object based on the configuration and geometry.

	Raises
	------
	ValueError
	If both t_inner and luminosity_requested are None.
	"""
	if config.montecarlo.enable_full_relativity:
	packet_source = BlackBodySimpleSourceRelativistic(
	base_seed=config.montecarlo.seed,
	time_explosion=config.supernova.time_explosion,
	)
	else:
	packet_source = BlackBodySimpleSource(base_seed=config.montecarlo.seed)

Bug t radiative #2560

Bug t radiative #2560

Conversation

Sumit112192 commented Mar 23, 2024 • edited Loading

📝 Description

📌 Resources

🚦 Testing

☑️ Checklist

andrewfullard Apr 15, 2024

Choose a reason for hiding this comment

andrewfullard Apr 15, 2024

Choose a reason for hiding this comment

Sumit112192 Apr 15, 2024

Choose a reason for hiding this comment

DeerWhale commented Apr 15, 2024

review-notebook-app bot commented May 2, 2024

Sumit112192 commented May 2, 2024

Sumit112192 commented May 2, 2024

Sumit112192 commented May 2, 2024

Sumit112192 commented May 2, 2024

codecov bot commented May 3, 2024

Codecov Report

andrewfullard commented May 3, 2024

tardis-bot commented May 3, 2024

Sumit112192 commented Mar 23, 2024 •

edited

Loading